virtual

Oops.

Done.

what will attr point to if Error happens?

The attr is always pointing to an outside variable. The pointer is not modified in Get method.

oh, so the value T* attr point to just not change, is that right?

The pointer attr is will never be changed inside Get method. The value *attr will not be changed if an error occurred. The value *attr will be set to correct value when there is no error.

What will happen if err == "Attribute Not Found" ?

If the error is Attribute Not Found, then the default value is used.

The default value is set in private data member, like float scale_{1.0};.

I think If the error is Attribute Not Found, then the default value is used. should be added to comment.
Another thing is that remind the user to set a default value for their attributes.

Done.

Background

Done.

Design Doc: Operator Attributes

Done.

In a neural network, each operator could contain some configurable attributes.

==>

Operators have attributes.

Done.

be of various types

Done.

The configurable attributes could be various types. Some operators need float value to configure; some need string value. We need a data structure to represent different types.

==>

Each attributes has a name and a type. We are going to describe them in a protobuf message.

Done.

Each operator contains different configurable attributes. The names of attributes are not same. We need an associate map from attribute name to attribute value for Operator.

==>

We plan to list attributes of an operator in an associate map: map<string, Attribute*>.

In protobuf, the type of Attributes is a map<string, Attribute> attrs; But in each op class, it stores simple strong typed values as member data.

class CosineOp {
public:
  void Init(const AttributeReader& reader) {
    scale_ = reader.Get<float>("scale")
  }

private:
  float scale_;
};

If we extract scale from protobuf map every time when we use it, it is very slow.

The ListValue message is defined as:

message ListValue {
     repeated int32 ints = 1;
     repeated float floats = 2;
     repeated string strings = 3;
 }

And GetArray method only has one template parameter T. So, how the GetArray get ListValue with various type.

Only one of ListValue field should be set. The protobuf 3 currently cannot make repeated field inside a oneof. So just make all ListValue a single message.

The tensorflow has a similar implementation

why named details ?

Because we need define some function that should not be used by the user but can only be defined in the header file.

So mark them in namespace details could tell the user not to use them directly.

It seems that most lib name this Contians
https://docs.oracle.com/javase/7/docs/api/java/util/List.html#contains(java.lang.Object)

Done.

need ";" at the end of this line

I found it. Done.

to be with the same name style with ATTR_READER_GETVALUE_IMPL and ATTR_READER_ISTYPE_IMPL

ATTR_GETARRAY_IMPL => ATTR_READER_GETARRAY_IMPL

just discuss, do we need a

T Get(const std::string& name, const T& default) const;

to set a default value

I see the design doc, people can use Contains to set a default value. So this is not a problem.

The oneof directive doesn't seem help much here -- it can locate a field in value but it cannot help us locate a field in ListValue. If our C++ code would still have to check which field is the one in ListValue, it would be easier just use the old plain syntax:

syntax = "proto3";

message Attribute {
  enum Type {
    INTS = 0;
    FLOATS = 1;
    STRINGS = 2;
    INT = 3;
    FLOAT = 4;
    STRING = 5;
  }
  Type type = 1;

  repeated int32 ints = 2;
  repeated float floats = 3;
  repeated string strings = 4;
  int32 int = 5;
  float float = 6;
  string string = 7;
}

syntax = "proto3";

message Attribute {
  repeated int32 ints = 1;
  repeated float floats = 2;
  repeated string strings = 3;
  optional int32 int = 4;
  optional float float = 5;
  optinoal string string = 6;
}

Maybe the Type is not useful, because we could use optional field and has_xxx() to detect AttributeType.

Surprise to know that Caffe2 searches attributes in a protobuf list. The right way is to load from protobuf field repeated Attrbribute attrs = x; into C++ data structure map<string, Attribute> and search in the C++ data structure.

It's my mistake, Caffe2 is loading all attribute into memory first.

OperatorDescription => OperatorDesc ?

AttributeReader => Attributes

This is not a reader, it holds and searches attributes as well.

What if an attribute was marked int in the protobuf message, but the user tries to retrieve its string value? How can we check and find such kind of errors?

Because we use optional, we can detect which type is set in protobuf, and assert fail when type mismatch.

Contains => IsType

For example,

if (IsType<std::vector<string>>("name")) {
  ...
}

Actually, maybe it would be easier to have Has and Type instead

class Attributes {
 public:
  bool Has(const std::string& name) const {
    return attrs_.find(name) != attrs_.end();
  }

  const std::type_info& Type() const(const std::string& name) const {
    PADDLE_ENFORCE(Has(name));
    switch (attrs_[name].type()) {
    case paddle::framework::Attribute::INTS:
      return typeid(int);
    ...
    }
  }
};

This doesn't look like a smart idea -- every time we change an attribute in the private: section, we must also change this function accordingly.

It seems that this document is not complete. I cannot find the part describing macros that define attributes of an operator?

Also, I am curious about how the Python API can fill in the protobuf field OperatorDesc::attrs, so that the C++ code could load.